]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #55515 - QuietMisdreavus:rustdoc-config, r=GuillaumeGomez
authorbors <bors@rust-lang.org>
Mon, 5 Nov 2018 09:48:46 +0000 (09:48 +0000)
committerbors <bors@rust-lang.org>
Mon, 5 Nov 2018 09:48:46 +0000 (09:48 +0000)
rustdoc: refactor: centralize all command-line argument parsing

This is something i've wanted to do for a while, since we keep having to add new arguments to places like `rust_input` or `core::run_core` whenever we add a new CLI flag or the like. Those functions have inflated up to 11-19, and in some cases hiding away the locations where some CLI flags were being parsed, obscuring their use. Now, we have a central place where all command-line configuration occurs, including argument validation.

One note about the design: i grouped together all the arguments that `html::render::run` needed, so that i could pass them on from compilation in one lump instead of trying to thread through individual items or clone the entire blob ahead of time.

One other thing this adds is that rustdoc also now recognizes all the `-Z` options that rustc does, since we were manually grabbing a few previously. Now we parse a full `DebuggingOptions` struct and hand it directly to rustc when scraping docs.

1  2 
src/librustdoc/html/render.rs
src/librustdoc/test.rs

index 8ba299d229885932e10da7ca1be49c3891178f7e,14512e8adf5fdc1b64865b49340283018ea5a78f..efd71ad0763e0e61abcbeb638f1e15c6118952d7
@@@ -52,11 -52,7 +52,7 @@@ use std::str
  use std::sync::Arc;
  use std::rc::Rc;
  
- use externalfiles::ExternalHtml;
  use errors;
- use getopts;
  use serialize::json::{ToJson, Json, as_json};
  use syntax::ast;
  use syntax::ext::base::MacroKind;
@@@ -70,6 -66,7 +66,7 @@@ use rustc::util::nodemap::{FxHashMap, F
  use rustc_data_structures::flock;
  
  use clean::{self, AttributesExt, GetDefId, SelfTy, Mutability};
+ use config::RenderOptions;
  use doctree;
  use fold::DocFolder;
  use html::escape::Escape;
@@@ -109,8 -106,6 +106,6 @@@ struct Context 
      /// The map used to ensure all generated 'id=' attributes are unique.
      id_map: Rc<RefCell<IdMap>>,
      pub shared: Arc<SharedContext>,
-     pub enable_index_page: bool,
-     pub index_page: Option<PathBuf>,
  }
  
  struct SharedContext {
@@@ -495,23 -490,25 +490,25 @@@ pub fn initial_ids() -> Vec<String> 
  
  /// Generates the documentation for `crate` into the directory `dst`
  pub fn run(mut krate: clean::Crate,
-            extern_urls: BTreeMap<String, String>,
-            external_html: &ExternalHtml,
-            playground_url: Option<String>,
-            dst: PathBuf,
-            resource_suffix: String,
+            options: RenderOptions,
             passes: FxHashSet<String>,
-            css_file_extension: Option<PathBuf>,
             renderinfo: RenderInfo,
-            sort_modules_alphabetically: bool,
-            themes: Vec<PathBuf>,
-            enable_minification: bool,
-            id_map: IdMap,
-            enable_index_page: bool,
-            index_page: Option<PathBuf>,
-            matches: &getopts::Matches,
-            diag: &errors::Handler,
- ) -> Result<(), Error> {
+            diag: &errors::Handler) -> Result<(), Error> {
+     // need to save a copy of the options for rendering the index page
+     let md_opts = options.clone();
+     let RenderOptions {
+         output,
+         external_html,
+         id_map,
+         playground_url,
+         sort_modules_alphabetically,
+         themes,
+         extension_css,
+         extern_html_root_urls,
+         resource_suffix,
+         ..
+     } = options;
      let src_root = match krate.src {
          FileName::Real(ref p) => match p.parent() {
              Some(p) => p.to_path_buf(),
          layout: layout::Layout {
              logo: String::new(),
              favicon: String::new(),
-             external_html: external_html.clone(),
+             external_html,
              krate: krate.name.clone(),
          },
-         css_file_extension,
+         css_file_extension: extension_css,
          created_dirs: Default::default(),
          sort_modules_alphabetically,
          themes,
              }
          }
      }
+     let dst = output;
      try_err!(fs::create_dir_all(&dst), &dst);
      krate = render_sources(&dst, &mut scx, krate)?;
      let cx = Context {
          codes: ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()),
          id_map: Rc::new(RefCell::new(id_map)),
          shared: Arc::new(scx),
-         enable_index_page,
-         index_page,
      };
  
      // Crawl the crate to build various caches used for the output
              },
              _ => PathBuf::new(),
          };
-         let extern_url = extern_urls.get(&e.name).map(|u| &**u);
+         let extern_url = extern_html_root_urls.get(&e.name).map(|u| &**u);
          cache.extern_locations.insert(n, (e.name.clone(), src_root,
                                            extern_location(e, extern_url, &cx.dst)));
  
      CACHE_KEY.with(|v| *v.borrow_mut() = cache.clone());
      CURRENT_LOCATION_KEY.with(|s| s.borrow_mut().clear());
  
-     write_shared(&cx, &krate, &*cache, index, enable_minification, matches, diag)?;
+     write_shared(&cx, &krate, &*cache, index, &md_opts, diag)?;
  
      // And finally render the whole crate's documentation
      cx.krate(krate)
@@@ -759,8 -755,7 +755,7 @@@ fn write_shared
      krate: &clean::Crate,
      cache: &Cache,
      search_index: String,
-     enable_minification: bool,
-     matches: &getopts::Matches,
+     options: &RenderOptions,
      diag: &errors::Handler,
  ) -> Result<(), Error> {
      // Write out the shared files. Note that these are shared among all rustdoc
  
      write_minify(cx.dst.join(&format!("rustdoc{}.css", cx.shared.resource_suffix)),
                   include_str!("static/rustdoc.css"),
-                  enable_minification)?;
+                  options.enable_minification)?;
      write_minify(cx.dst.join(&format!("settings{}.css", cx.shared.resource_suffix)),
                   include_str!("static/settings.css"),
-                  enable_minification)?;
+                  options.enable_minification)?;
  
      // To avoid "light.css" to be overwritten, we'll first run over the received themes and only
      // then we'll run over the "official" styles.
            include_bytes!("static/wheel.svg"))?;
      write_minify(cx.dst.join(&format!("light{}.css", cx.shared.resource_suffix)),
                   include_str!("static/themes/light.css"),
-                  enable_minification)?;
+                  options.enable_minification)?;
      themes.insert("light".to_owned());
      write_minify(cx.dst.join(&format!("dark{}.css", cx.shared.resource_suffix)),
                   include_str!("static/themes/dark.css"),
-                  enable_minification)?;
+                  options.enable_minification)?;
      themes.insert("dark".to_owned());
  
      let mut themes: Vec<&String> = themes.iter().collect();
@@@ -860,10 -855,10 +855,10 @@@ themePicker.onblur = handleThemeButtons
  
      write_minify(cx.dst.join(&format!("main{}.js", cx.shared.resource_suffix)),
                   include_str!("static/main.js"),
-                  enable_minification)?;
+                  options.enable_minification)?;
      write_minify(cx.dst.join(&format!("settings{}.js", cx.shared.resource_suffix)),
                   include_str!("static/settings.js"),
-                  enable_minification)?;
+                  options.enable_minification)?;
  
      {
          let mut data = format!("var resourcesSuffix = \"{}\";\n",
          data.push_str(include_str!("static/storage.js"));
          write_minify(cx.dst.join(&format!("storage{}.js", cx.shared.resource_suffix)),
                       &data,
-                      enable_minification)?;
+                      options.enable_minification)?;
      }
  
      if let Some(ref css) = cx.shared.css_file_extension {
          let out = cx.dst.join(&format!("theme{}.css", cx.shared.resource_suffix));
-         if !enable_minification {
+         if !options.enable_minification {
              try_err!(fs::copy(css, out), css);
          } else {
              let mut f = try_err!(File::open(css), css);
              let mut buffer = String::with_capacity(1000);
  
              try_err!(f.read_to_string(&mut buffer), css);
-             write_minify(out, &buffer, enable_minification)?;
+             write_minify(out, &buffer, options.enable_minification)?;
          }
      }
      write_minify(cx.dst.join(&format!("normalize{}.css", cx.shared.resource_suffix)),
                   include_str!("static/normalize.css"),
-                  enable_minification)?;
+                  options.enable_minification)?;
      write(cx.dst.join("FiraSans-Regular.woff"),
            include_bytes!("static/FiraSans-Regular.woff"))?;
      write(cx.dst.join("FiraSans-Medium.woff"),
      let mut w = try_err!(File::create(&dst), &dst);
      try_err!(writeln!(&mut w, "var N = null;var searchIndex = {{}};"), &dst);
      for index in &all_indexes {
-         try_err!(write_minify_replacer(&mut w, &*index, enable_minification,
+         try_err!(write_minify_replacer(&mut w, &*index, options.enable_minification,
                                         &[(minifier::js::Keyword::Null, "N")]),
                   &dst);
      }
      try_err!(writeln!(&mut w, "initSearch(searchIndex);"), &dst);
  
-     if cx.enable_index_page == true {
-         if let Some(ref index_page) = cx.index_page {
-             ::markdown::render(index_page,
-                                cx.dst.clone(),
-                                &matches, &(*cx.shared).layout.external_html,
-                                !matches.opt_present("markdown-no-toc"),
-                                diag);
+     if options.enable_index_page {
+         if let Some(index_page) = options.index_page.clone() {
+             let mut md_opts = options.clone();
+             md_opts.output = cx.dst.clone();
+             md_opts.external_html = (*cx.shared).layout.external_html.clone();
+             ::markdown::render(index_page, md_opts, diag);
          } else {
              let dst = cx.dst.join("index.html");
              let mut w = BufWriter::new(try_err!(File::create(&dst), &dst));
@@@ -2322,8 -2317,8 +2317,8 @@@ fn document(w: &mut fmt::Formatter, cx
      if let Some(ref name) = item.name {
          info!("Documenting {}", name);
      }
 -    document_stability(w, cx, item)?;
 -    document_full(w, item, cx, "")?;
 +    document_stability(w, cx, item, false)?;
 +    document_full(w, item, cx, "", false)?;
      Ok(())
  }
  
@@@ -2332,19 -2327,15 +2327,19 @@@ fn render_markdown(w: &mut fmt::Formatt
                     cx: &Context,
                     md_text: &str,
                     links: Vec<(String, String)>,
 -                   prefix: &str)
 +                   prefix: &str,
 +                   is_hidden: bool)
                     -> fmt::Result {
      let mut ids = cx.id_map.borrow_mut();
 -    write!(w, "<div class='docblock'>{}{}</div>",
 -        prefix, Markdown(md_text, &links, RefCell::new(&mut ids), cx.codes))
 +    write!(w, "<div class='docblock{}'>{}{}</div>",
 +           if is_hidden { " hidden" } else { "" },
 +           prefix,
 +           Markdown(md_text, &links, RefCell::new(&mut ids),
 +           cx.codes))
  }
  
  fn document_short(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item, link: AssocItemLink,
 -                  prefix: &str) -> fmt::Result {
 +                  prefix: &str, is_hidden: bool) -> fmt::Result {
      if let Some(s) = item.doc_value() {
          let markdown = if s.contains('\n') {
              format!("{} [Read more]({})",
          } else {
              plain_summary_line(Some(s))
          };
 -        render_markdown(w, cx, &markdown, item.links(), prefix)?;
 +        render_markdown(w, cx, &markdown, item.links(), prefix, is_hidden)?;
      } else if !prefix.is_empty() {
 -        write!(w, "<div class='docblock'>{}</div>", prefix)?;
 +        write!(w, "<div class='docblock{}'>{}</div>",
 +               if is_hidden { " hidden" } else { "" },
 +               prefix)?;
      }
      Ok(())
  }
  
  fn document_full(w: &mut fmt::Formatter, item: &clean::Item,
 -                 cx: &Context, prefix: &str) -> fmt::Result {
 +                 cx: &Context, prefix: &str, is_hidden: bool) -> fmt::Result {
      if let Some(s) = cx.shared.maybe_collapsed_doc_value(item) {
          debug!("Doc block: =====\n{}\n=====", s);
 -        render_markdown(w, cx, &*s, item.links(), prefix)?;
 +        render_markdown(w, cx, &*s, item.links(), prefix, is_hidden)?;
      } else if !prefix.is_empty() {
 -        write!(w, "<div class='docblock'>{}</div>", prefix)?;
 +        write!(w, "<div class='docblock{}'>{}</div>",
 +               if is_hidden { " hidden" } else { "" },
 +               prefix)?;
      }
      Ok(())
  }
  
 -fn document_stability(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item) -> fmt::Result {
 +fn document_stability(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item,
 +                      is_hidden: bool) -> fmt::Result {
      let stabilities = short_stability(item, cx, true);
      if !stabilities.is_empty() {
 -        write!(w, "<div class='stability'>")?;
 +        write!(w, "<div class='stability{}'>", if is_hidden { " hidden" } else { "" })?;
          for stability in stabilities {
              write!(w, "{}", stability)?;
          }
@@@ -3943,21 -3929,14 +3938,21 @@@ fn render_impl(w: &mut fmt::Formatter, 
              RenderMode::ForDeref { mut_: deref_mut_ } => should_render_item(&item, deref_mut_),
          };
  
 +        let (is_hidden, extra_class) = if trait_.is_none() ||
 +                                          item.doc_value().is_some() ||
 +                                          item.inner.is_associated() {
 +            (false, "")
 +        } else {
 +            (true, " hidden")
 +        };
          match item.inner {
              clean::MethodItem(clean::Method { ref decl, .. }) |
 -            clean::TyMethodItem(clean::TyMethod{ ref decl, .. }) => {
 +            clean::TyMethodItem(clean::TyMethod { ref decl, .. }) => {
                  // Only render when the method is not static or we allow static methods
                  if render_method_item {
                      let id = cx.derive_id(format!("{}.{}", item_type, name));
                      let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
 -                    write!(w, "<h4 id='{}' class=\"{}\">", id, item_type)?;
 +                    write!(w, "<h4 id='{}' class=\"{}{}\">", id, item_type, extra_class)?;
                      write!(w, "{}", spotlight_decl(decl)?)?;
                      write!(w, "<span id='{}' class='invisible'>", ns_id)?;
                      write!(w, "<table class='table-display'><tbody><tr><td><code>")?;
              clean::TypedefItem(ref tydef, _) => {
                  let id = cx.derive_id(format!("{}.{}", ItemType::AssociatedType, name));
                  let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
 -                write!(w, "<h4 id='{}' class=\"{}\">", id, item_type)?;
 +                write!(w, "<h4 id='{}' class=\"{}{}\">", id, item_type, extra_class)?;
                  write!(w, "<span id='{}' class='invisible'><code>", ns_id)?;
                  assoc_type(w, item, &Vec::new(), Some(&tydef.type_), link.anchor(&id))?;
                  write!(w, "</code></span></h4>\n")?;
              clean::AssociatedConstItem(ref ty, ref default) => {
                  let id = cx.derive_id(format!("{}.{}", item_type, name));
                  let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
 -                write!(w, "<h4 id='{}' class=\"{}\">", id, item_type)?;
 +                write!(w, "<h4 id='{}' class=\"{}{}\">", id, item_type, extra_class)?;
                  write!(w, "<span id='{}' class='invisible'><code>", ns_id)?;
                  assoc_const(w, item, ty, default.as_ref(), link.anchor(&id))?;
                  let src = if let Some(l) = (Item { cx, item }).src_href() {
              clean::AssociatedTypeItem(ref bounds, ref default) => {
                  let id = cx.derive_id(format!("{}.{}", item_type, name));
                  let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
 -                write!(w, "<h4 id='{}' class=\"{}\">", id, item_type)?;
 +                write!(w, "<h4 id='{}' class=\"{}{}\">", id, item_type, extra_class)?;
                  write!(w, "<span id='{}' class='invisible'><code>", ns_id)?;
                  assoc_type(w, item, bounds, default.as_ref(), link.anchor(&id))?;
                  write!(w, "</code></span></h4>\n")?;
                      if let Some(it) = t.items.iter().find(|i| i.name == item.name) {
                          // We need the stability of the item from the trait
                          // because impls can't have a stability.
 -                        document_stability(w, cx, it)?;
 +                        document_stability(w, cx, it, is_hidden)?;
                          if item.doc_value().is_some() {
 -                            document_full(w, item, cx, "")?;
 +                            document_full(w, item, cx, "", is_hidden)?;
                          } else if show_def_docs {
                              // In case the item isn't documented,
                              // provide short documentation from the trait.
 -                            document_short(w, cx, it, link, "")?;
 +                            document_short(w, cx, it, link, "", is_hidden)?;
                          }
                      }
                  } else {
 -                    document_stability(w, cx, item)?;
 +                    document_stability(w, cx, item, is_hidden)?;
                      if show_def_docs {
 -                        document_full(w, item, cx, "")?;
 +                        document_full(w, item, cx, "", is_hidden)?;
                      }
                  }
              } else {
 -                document_stability(w, cx, item)?;
 +                document_stability(w, cx, item, is_hidden)?;
                  if show_def_docs {
 -                    document_short(w, cx, item, link, "")?;
 +                    document_short(w, cx, item, link, "", is_hidden)?;
                  }
              }
          }
diff --combined src/librustdoc/test.rs
index 06cb4fbd71602787933804c4a9375caa888723bd,ebc26d73c4553cbc8d33cfdc7d716bdba5ba77ed..d9bab91fd0c7892b8b79e70333f7a16b84035e90
@@@ -12,7 -12,7 +12,7 @@@ use std::env
  use std::ffi::OsString;
  use std::io::prelude::*;
  use std::io;
- use std::path::{Path, PathBuf};
+ use std::path::PathBuf;
  use std::panic::{self, AssertUnwindSafe};
  use std::process::Command;
  use std::str;
@@@ -42,6 -42,7 +42,7 @@@ use errors
  use errors::emitter::ColorConfig;
  
  use clean::Attributes;
+ use config::Options;
  use html::markdown::{self, ErrorCodes, LangString};
  
  #[derive(Clone, Default)]
@@@ -55,34 -56,23 +56,23 @@@ pub struct TestOptions 
      pub attrs: Vec<String>,
  }
  
- pub fn run(input_path: &Path,
-            cfgs: Vec<String>,
-            libs: SearchPaths,
-            externs: Externs,
-            mut test_args: Vec<String>,
-            crate_name: Option<String>,
-            maybe_sysroot: Option<PathBuf>,
-            display_warnings: bool,
-            linker: Option<PathBuf>,
-            edition: Edition,
-            cg: CodegenOptions)
-            -> isize {
-     let input = config::Input::File(input_path.to_owned());
+ pub fn run(mut options: Options) -> isize {
+     let input = config::Input::File(options.input.clone());
  
      let sessopts = config::Options {
-         maybe_sysroot: maybe_sysroot.clone().or_else(
+         maybe_sysroot: options.maybe_sysroot.clone().or_else(
              || Some(env::current_exe().unwrap().parent().unwrap().parent().unwrap().to_path_buf())),
-         search_paths: libs.clone(),
+         search_paths: options.libs.clone(),
          crate_types: vec![config::CrateType::Dylib],
-         cg: cg.clone(),
-         externs: externs.clone(),
+         cg: options.codegen_options.clone(),
+         externs: options.externs.clone(),
          unstable_features: UnstableFeatures::from_environment(),
          lint_cap: Some(::rustc::lint::Level::Allow),
          actually_rustdoc: true,
          debugging_opts: config::DebuggingOptions {
              ..config::basic_debugging_options()
          },
-         edition,
+         edition: options.edition,
          ..config::Options::default()
      };
      driver::spawn_thread_pool(sessopts, |sessopts| {
                                              Some(source_map.clone()));
  
          let mut sess = session::build_session_(
-             sessopts, Some(input_path.to_owned()), handler, source_map.clone(),
+             sessopts, Some(options.input), handler, source_map.clone(),
          );
          let codegen_backend = rustc_driver::get_codegen_backend(&sess);
          let cstore = CStore::new(codegen_backend.metadata_loader());
          rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
  
-         let mut cfg = config::build_configuration(&sess, config::parse_cfgspecs(cfgs.clone()));
+         let mut cfg = config::build_configuration(&sess,
+                                                   config::parse_cfgspecs(options.cfgs.clone()));
          target_features::add_configuration(&mut cfg, &sess, &*codegen_backend);
          sess.parse_sess.config = cfg;
  
              ).expect("phase_2_configure_and_expand aborted in rustdoc!")
          };
  
-         let crate_name = crate_name.unwrap_or_else(|| {
+         let crate_name = options.crate_name.unwrap_or_else(|| {
              ::rustc_codegen_utils::link::find_crate_name(None, &hir_forest.krate().attrs, &input)
          });
          let mut opts = scrape_test_config(hir_forest.krate());
-         opts.display_warnings |= display_warnings;
+         opts.display_warnings |= options.display_warnings;
          let mut collector = Collector::new(
              crate_name,
-             cfgs,
-             libs,
-             cg,
-             externs,
+             options.cfgs,
+             options.libs,
+             options.codegen_options,
+             options.externs,
              false,
              opts,
-             maybe_sysroot,
+             options.maybe_sysroot,
              Some(source_map),
-              None,
-             linker,
-             edition
+             None,
+             options.linker,
+             options.edition
          );
  
          {
              });
          }
  
-         test_args.insert(0, "rustdoctest".to_string());
+         options.test_args.insert(0, "rustdoctest".to_string());
  
-         testing::test_main(&test_args,
+         testing::test_main(&options.test_args,
                          collector.tests.into_iter().collect(),
-                         testing::Options::new().display_output(display_warnings));
+                         testing::Options::new().display_output(options.display_warnings));
          0
      })
  }
@@@ -378,7 -369,7 +369,7 @@@ pub fn make_test(s: &str
                   dont_insert_main: bool,
                   opts: &TestOptions)
                   -> (String, usize) {
 -    let (crate_attrs, everything_else) = partition_source(s);
 +    let (crate_attrs, everything_else, crates) = partition_source(s);
      let everything_else = everything_else.trim();
      let mut line_offset = 0;
      let mut prog = String::new();
      // are intended to be crate attributes.
      prog.push_str(&crate_attrs);
  
 +    // Uses libsyntax to parse the doctest and find if there's a main fn and the extern
 +    // crate already is included.
 +    let (already_has_main, already_has_extern_crate) = crate::syntax::with_globals(|| {
 +        use crate::syntax::{ast, parse::{self, ParseSess}, source_map::FilePathMapping};
 +        use crate::syntax_pos::FileName;
 +        use errors::emitter::EmitterWriter;
 +        use errors::Handler;
 +
 +        let filename = FileName::Anon;
 +        let source = crates + &everything_else;
 +
 +        // any errors in parsing should also appear when the doctest is compiled for real, so just
 +        // send all the errors that libsyntax emits directly into a Sink instead of stderr
 +        let cm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
 +        let emitter = EmitterWriter::new(box io::sink(), None, false, false);
 +        let handler = Handler::with_emitter(false, false, box emitter);
 +        let sess = ParseSess::with_span_handler(handler, cm);
 +
 +        debug!("about to parse: \n{}", source);
 +
 +        let mut found_main = false;
 +        let mut found_extern_crate = cratename.is_none();
 +
 +        let mut parser = match parse::maybe_new_parser_from_source_str(&sess, filename, source) {
 +            Ok(p) => p,
 +            Err(errs) => {
 +                for mut err in errs {
 +                    err.cancel();
 +                }
 +
 +                return (found_main, found_extern_crate);
 +            }
 +        };
 +
 +        loop {
 +            match parser.parse_item() {
 +                Ok(Some(item)) => {
 +                    if !found_main {
 +                        if let ast::ItemKind::Fn(..) = item.node {
 +                            if item.ident.as_str() == "main" {
 +                                found_main = true;
 +                            }
 +                        }
 +                    }
 +
 +                    if !found_extern_crate {
 +                        if let ast::ItemKind::ExternCrate(original) = item.node {
 +                            // This code will never be reached if `cratename` is none because
 +                            // `found_extern_crate` is initialized to `true` if it is none.
 +                            let cratename = cratename.unwrap();
 +
 +                            match original {
 +                                Some(name) => found_extern_crate = name.as_str() == cratename,
 +                                None => found_extern_crate = item.ident.as_str() == cratename,
 +                            }
 +                        }
 +                    }
 +
 +                    if found_main && found_extern_crate {
 +                        break;
 +                    }
 +                }
 +                Ok(None) => break,
 +                Err(mut e) => {
 +                    e.cancel();
 +                    break;
 +                }
 +            }
 +        }
 +
 +        (found_main, found_extern_crate)
 +    });
 +
      // Don't inject `extern crate std` because it's already injected by the
      // compiler.
 -    if !s.contains("extern crate") && !opts.no_crate_inject && cratename != Some("std") {
 +    if !already_has_extern_crate && !opts.no_crate_inject && cratename != Some("std") {
          if let Some(cratename) = cratename {
 +            // Make sure its actually used if not included.
              if s.contains(cratename) {
                  prog.push_str(&format!("extern crate {};\n", cratename));
                  line_offset += 1;
          }
      }
  
 -    // FIXME (#21299): prefer libsyntax or some other actual parser over this
 -    // best-effort ad hoc approach
 -    let already_has_main = s.lines()
 -        .map(|line| {
 -            let comment = line.find("//");
 -            if let Some(comment_begins) = comment {
 -                &line[0..comment_begins]
 -            } else {
 -                line
 -            }
 -        })
 -        .any(|code| code.contains("fn main"));
 -
      if dont_insert_main || already_has_main {
          prog.push_str(everything_else);
      } else {
  }
  
  // FIXME(aburka): use a real parser to deal with multiline attributes
 -fn partition_source(s: &str) -> (String, String) {
 +fn partition_source(s: &str) -> (String, String, String) {
      let mut after_header = false;
      let mut before = String::new();
 +    let mut crates = String::new();
      let mut after = String::new();
  
      for line in s.lines() {
              after.push_str(line);
              after.push_str("\n");
          } else {
 +            if trimline.starts_with("#[macro_use] extern crate")
 +                || trimline.starts_with("extern crate") {
 +                crates.push_str(line);
 +                crates.push_str("\n");
 +            }
              before.push_str(line);
              before.push_str("\n");
          }
      }
  
 -    (before, after)
 +    (before, after, crates)
  }
  
  pub trait Tester {
@@@ -1081,38 -1005,4 +1072,38 @@@ assert_eq!(2+2, 4)
          let output = make_test(input, None, false, &opts);
          assert_eq!(output, (expected, 1));
      }
 +
 +    #[test]
 +    fn make_test_issues_21299_33731() {
 +        let opts = TestOptions::default();
 +
 +        let input =
 +"// fn main
 +assert_eq!(2+2, 4);";
 +
 +        let expected =
 +"#![allow(unused)]
 +fn main() {
 +// fn main
 +assert_eq!(2+2, 4);
 +}".to_string();
 +
 +        let output = make_test(input, None, false, &opts);
 +        assert_eq!(output, (expected, 2));
 +
 +        let input =
 +"extern crate hella_qwop;
 +assert_eq!(asdf::foo, 4);";
 +
 +        let expected =
 +"#![allow(unused)]
 +extern crate hella_qwop;
 +extern crate asdf;
 +fn main() {
 +assert_eq!(asdf::foo, 4);
 +}".to_string();
 +
 +        let output = make_test(input, Some("asdf"), false, &opts);
 +        assert_eq!(output, (expected, 3));
 +    }
  }